Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix all bits broken by viper API changes #5982

Merged
merged 8 commits into from
Apr 14, 2020
Merged

Conversation

alessio
Copy link
Contributor

@alessio alessio commented Apr 10, 2020

github.com/spf13/viper's recent releases introduced a semantic
change in some public API such as viper.IsSet(), which have
broken some of our flags checks. Instead of checking whether
users have changed a flag's default value we should rely on such
defaults and adjust runtime behaviour accordingly. In order to do
so, it's important that we pick sane defaults for all our flags.

The --pruning flag and configuration option now allow for a
fake custom strategy. When users elect custom, then the
pruning-{keep,snapshot}-every options are interpreted and
parsed; else they're ignored.
Zero is pruning-{keep,snapshot}-every default value. When
users choose to set a custom pruning strategy they are
signalling that they want more fine-grainted control, therefore
it's legitimate to expect them to know what they are doing and
enter valid values for both options.

Closes: #5964


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Apr 14, 2020

Codecov Report

Merging #5982 into master will decrease coverage by 0.02%.
The diff coverage is 88.46%.

@@            Coverage Diff             @@
##           master    #5982      +/-   ##
==========================================
- Coverage   58.27%   58.25%   -0.03%     
==========================================
  Files         396      396              
  Lines       23595    23603       +8     
==========================================
- Hits        13750    13749       -1     
- Misses       8883     8893      +10     
+ Partials      962      961       -1     

[skip ci]
@alessio alessio marked this pull request as ready for review April 14, 2020 13:28
@alessio alessio added the R4R label Apr 14, 2020
@alessio alessio requested review from jgimeno and tac0turtle April 14, 2020 13:28
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK. Thanks @alessio.

Copy link
Contributor

@jgimeno jgimeno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one!! :D

@alessio alessio merged commit e8cedf2 into master Apr 14, 2020
@alessio alessio deleted the alessio/dont-use-viper-isset branch April 14, 2020 15:24
@alessio alessio mentioned this pull request Apr 21, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query issue when upgrading from v0.37.3 to v0.37.8
3 participants